Api enhancement to support optional collection_class parameter #13845
Conversation
- Allows subclassing any collection - specified via ?collection_class=<class> i.e. GET /api/vms?collection_class=ManageIQ::Providers::CloudManager::Vm - Updated CLI to support --collection-class=<class>
|
cc @gtanzillo |
| # If it is the collection's class, then we're good to go as is. | ||
| return if param_klass == collection_klass | ||
|
|
||
| if param_klass < collection_klass |
There was a problem hiding this comment.
I believe you can do <= and just collapse this conditional with the one above.
| return unless param.present? | ||
|
|
||
| begin | ||
| param_klass = param.constantize |
There was a problem hiding this comment.
Prefer safe_constantize over constantize for this case. and don't rescue (instead just check for nil).
|
@abellotti Want would a caller look like (i.e. what would a get and a post look like)? |
- Simplified validate_collection_class logic - Leveraging descendants to keep brakeman happy - Added rspecs
abellotti
force-pushed
the
api_collection_class
branch
from
d820026
to
dd8fd54
Compare
|
Checked commits abellotti/manageiq@97df523~...dd8fd54 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0 |
| klass = collection_class(@req.collection) | ||
| return if param == klass.name | ||
|
|
||
| param_klass = klass.descendants.detect { |sub_klass| param == sub_klass.name } |
There was a problem hiding this comment.
had no choice :) brakeman wasn't happy with constantize with user specified parameter. Thanks.
There was a problem hiding this comment.
Yeah, that's usually a big no-no, which was why I was commenting. Glad you found a much nicer alternative!
abellotti
changed the title
i.e. GET /api/vms?collection_class=ManageIQ::Providers::CloudManager::Vm
https://www.pivotaltracker.com/story/show/139478269